HDDS-15064. [STS] Artifacts for Ranger to Consider S3 Action when Authorizing#10108
HDDS-15064. [STS] Artifacts for Ranger to Consider S3 Action when Authorizing#10108ChenSammi merged 3 commits intoapache:masterfrom
Conversation
|
hi @ChenSammi - this is ready to review when you have a chance. Thanks! |
| private final Set<IAccessAuthorizer.ACLType> permissions; | ||
| /** | ||
| * S3 action names (e.g. s3:GetObject) from the session policy. When present, the permissions will be further | ||
| * restricted by the set of available S3 actions. An empty (or null) set means all actions granted by the |
There was a problem hiding this comment.
means all actions granted by the permissions are allowed
This implies that each permission maps to specific actions. While this is true at S3 gateway implementation, this is not formally documented, right? I suggest replacing this text with:
means the grant does not enforce any restructions on actions
There was a problem hiding this comment.
updated - eventually there will be documentation
| private final Set<IOzoneObj> objects; | ||
| private final Set<IAccessAuthorizer.ACLType> permissions; | ||
| /** | ||
| * S3 action names (e.g. s3:GetObject) from the session policy. When present, the permissions will be further |
There was a problem hiding this comment.
Given the field name is s3Actions, I suggest dropping prefix s3: in the values.
There was a problem hiding this comment.
updated (made corresponding change in RequestContext as well)
| private final Set<String> s3Actions; | ||
|
|
||
| public OzoneGrant(Set<IOzoneObj> objects, Set<IAccessAuthorizer.ACLType> permissions) { | ||
| this(objects, permissions, Collections.emptySet()); |
There was a problem hiding this comment.
line 113 creates a new LinedHashSet instance and a Collections.unmodifiable instance from Collections.emptySet() passed here. Consider avoiding this by directly assigning members here:
this.objects = objects;
this.permission = permissions;
this.s3Actions = Collections.emptySet();
| assertEquals(permissions, grant.getPermissions()); | ||
| assertEquals(s3Actions, grant.getS3Actions()); | ||
| // Ensure the s3 actions are not modifiable | ||
| assertThrows(UnsupportedOperationException.class, () -> grant.getS3Actions().add("s3:GetObject")); |
There was a problem hiding this comment.
Remove "s3:" prefix, from here and other occurrences as well?
Conflicts: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/AssumeRoleRequest.java hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/RequestContext.java hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestRequestContext.java
mneethiraj
left a comment
mneethiraj
left a comment
There was a problem hiding this comment.
Looks good from Ranger integration pov.
|
Thanks @fmorg-git , and @mneethiraj for the review. |
3 participants
Please describe your PR in detail:
s3:PutObjectTaggingors3:DeleteObjectTagging. Similarly, becauses3:PutObjectrequires read on volume, read on bucket, and create and write on key, someone withs3:PutObjectaccess can also calls3:PutObjectTagging(as an example). To prevent having more access than requested (or different access than requested), we need a means of restricting the ACL permissions further by S3 actions.To do this, we introduce an s3Action field in RequestContext so that if populated, the RangerOzoneAuthorizer would further restrict the permissions according to the S3 action.
Additionally, the OzoneGrant would contain a Set representing the S3 actions that are allowed for an inline policy. If all actions are allowed, then the Set would be empty (or null).
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-15064
How was this patch tested?
unit tests